Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use dbplyr to handle more of our *_joins #157

Closed
wants to merge 17 commits into from

Conversation

RasmusSkytte
Copy link
Contributor

@RasmusSkytte RasmusSkytte commented Oct 15, 2024

Intent

This PR introduces a large change to our *_join functions.

Before, we manually handled selection of columns and the translation from == to identical for columns with NA.

Now, we instead piggyback on dbplyr translations to handle this for us.

This should ensure we cover many more edge cases. In addition, we now also have good implementations of
full_join, anti_join and semi_join.

Approach

Instead of creating our overwritten joins via the sql_on argument and a subsequent select statement, we now create a special by argument that uses the a combination of na_matches = "never" (the default, non NA matching option) and na_matches = "na" to construct a join that selectively joins NA with NA.

This means many of our old helper functions (select helper and sql_on helper) are now replaced with helpers that handles dplyr::join_by() objects.

The new combined by argument does break the select statement of the dbplyr translation, so we have to manually fix it.

Known issues

This PR builds on un-merged PR

Checklist

  • The PR passes all local unit tests
  • I have documented any new features introduced
  • If the PR adds a new feature, please add an entry in NEWS.md
  • A reviewer is assigned to this PR

@RasmusSkytte RasmusSkytte self-assigned this Oct 15, 2024
@RasmusSkytte RasmusSkytte added the enhancement New feature or request label Oct 15, 2024
@RasmusSkytte
Copy link
Contributor Author

The SQL Server translation is too indirect for this approach to work.
Closing for now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant